Infotip - Clear earlier infotip content field while creating the next one#57
Conversation
Preview via Cloudflare R2 Storage⚡️WordPress Playground Preview I will update this comment with the latest preview links as you push more changes to this PR. Note The preview sites are created using WordPress Playground. You can add content, edit settings, and test the pull request as you would on a real site, but please note that changes are not saved between sessions. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the infotip inline UI and web component to extract constants and handlers, add clear/reset functionality in each tab, and modularize rendering and update logic.
- Extracted
PLACEMENT_OPTIONSandICONSconstants; replaced inline lambdas with named handlers in Text, Overlay, and Icon tabs - Fixed
removeAttributesto avoid mutatingactiveAttributesand introduced default-type assignment - Modularized
infotip-web-component.jswithrenderElement,updateIcon,positionArrow, and a switch-basedattributeChangedCallback
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/infotip/inline-ui.js | Extracted constants, DRY’d out handlers, fixed attribute mutation, added clear/reset handlers |
| assets/infotip/infotip-web-component.js | Broke out template, icon, position and style logic into methods; unified attribute change logic |
Comments suppressed due to low confidence (2)
src/infotip/inline-ui.js:164
- [nitpick] Variable
overLaySettingsEnabledhas inconsistent casing for “overlay”; consider renaming tooverlaySettingsEnabledfor clarity and consistency.
const overLaySettingsEnabled =
src/infotip/inline-ui.js:117
- [nitpick] New clear/reset handlers have been introduced; ensure you add or update unit tests to cover
handleClear,handleReset, and similar reset flows.
const handleClear = () => {
| if ( iconEnabled && ! activeAttributes[ 'icon-type' ] ) { | ||
| updateAttributes( { | ||
| 'icon-type': 'info', | ||
| } ); | ||
| updateAttributes( { 'icon-type': 'info' } ); | ||
| } |
There was a problem hiding this comment.
Avoid calling updateAttributes during render; side effects in render can trigger infinite loops. Move the default icon-type assignment into a useEffect or initialization step.
nagpai
changed the title
nagpai
requested review from
Verma-Punit and
ajitbohra
and removed request for
ajitbohra
Verma-Punit
left a comment
Verma-Punit
left a comment
There was a problem hiding this comment.
@nagpai Looks Good ✨
Summary
Fixes the bug within the editor, where text content from an earlier tooltip persisted within the popover, when we moved to the next tooltip.
Fixes another bug where the earlier tooltip remained open when subsequent tooltips are added.
PR adds a useEffect to hide all tooltips before adding a new one.
Related Issue
#57
Steps to test
Screenshots / Screen Recording / Logs
infotip-clearup-test-1751624920462.mp4